Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧲 Update vscalc() function and add to documentation #3520

Merged
merged 31 commits into from
Feb 17, 2025

Conversation

chris-ashe
Copy link
Collaborator

@chris-ashe chris-ashe commented Jan 30, 2025

Description

The vscalc() function has now been improved and added to the documentation.

  • New unit types has been added to the style guide to cover magnetic fluxes. Now the prefix web_ or vs_ can be used depending what better suites the context
  • The style guide recommendation for inductance prefixes has been changed from h_ to ind_
  • Removed inputting the value of vacuum permeability in to the function and instead call from constants.f90
  • The docs for the plasma resistive heating section have been updated to show all steps of plasma_ohmic_heating(), this also includes a boundary fix for the neo-classical enhancement for the resistivity that is only true for $A = 2.5-4.0$.

Old output:

image

New output:

image

The bootstrap current block has been moved below the volt-second section

Namespace changes

Functions

vscalc() -> calculate_volt_second_requirements()


Variables

  • gamma -> ejima_coeff: Very generic name that provided no insight
  • phiint -> vs_plasma_internal : This has now been added to output.
  • vsbrn -> vs_plasma_burn_required
  • vsstt -> vs_plasma_total_required
  • lpulse -> i_pulsed_plant: Changed prefix to new i_ style
  • vsres -> vs_plasma_res_ramp : Didnt specify the flux in what system and when the consumption occured
  • vsind -> vs_plasma_ind_ramp: Didnt specify the flux in what system and when the consumption occured
  • vburn -> v_plasma_loop_burn: Didnt specify system or where the voltage was measured.
  • rlp -> ind_plasma: Very generic name that provided no insight
  • rli -> ind_plasma_internal_norm: Very generic name that provided no insight
  • rpfac -> f_plasma_res_neo

✨ New additions

  • v_plasma_loop_burn: Wascalculated directly in the output section. Is now a tracked variable in physics_variables.f90. Value is returned in calculate_volt_second_requirements() and has been added to test_calculate_volt_second_requirements()

🐛 Bugs

  • The neoclassical resistivity enhancement factor for the plasma resistance was always applied even though the fit is only valid for aspect ratios of >2.5 and <4.
# The expression is valid for aspect ratios in the range 2.5 to 4.0
        rpfac = 4.3 - 0.6 * rmajor / rminor

Checklist

I confirm that I have completed the following checks:

  • My changes follow the PROCESS style guide
  • I have justified any large differences in the regression tests caused by this pull request in the comments.
  • I have added new tests where appropriate for the changes I have made.
  • If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments.
  • If I have made documentation changes, I have checked they render correctly.
  • I have added documentation for my change, if appropriate.

@chris-ashe chris-ashe added the Physics Relating to the physics models label Jan 30, 2025
@chris-ashe chris-ashe self-assigned this Jan 30, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 61.70213% with 18 lines in your changes missing coverage. Please review.

Project coverage is 31.54%. Comparing base (8cc0cf6) to head (d9ab030).

Files with missing lines Patch % Lines
process/physics.py 58.82% 14 Missing ⚠️
process/pfcoil.py 0.00% 2 Missing ⚠️
process/io/plot_proc.py 0.00% 1 Missing ⚠️
process/plasma_geometry.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3520      +/-   ##
==========================================
+ Coverage   31.48%   31.54%   +0.05%     
==========================================
  Files          82       82              
  Lines       19562    19567       +5     
==========================================
+ Hits         6160     6173      +13     
+ Misses      13402    13394       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chris-ashe chris-ashe force-pushed the update_vscalc_function_and_add_to_docs branch from aa1f2c8 to 12d8b00 Compare January 31, 2025 10:57
@chris-ashe chris-ashe force-pushed the update_vscalc_function_and_add_to_docs branch 5 times, most recently from 0deb114 to fe4d28e Compare February 11, 2025 10:37
@chris-ashe chris-ashe marked this pull request as ready for review February 11, 2025 10:41
Copy link
Collaborator

@j-a-foster j-a-foster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor spelling correction but otherwise happy with the docs.

@chris-ashe chris-ashe requested review from j-a-foster and jonmaddock and removed request for timothy-nunn February 13, 2025 15:57
@timothy-nunn timothy-nunn requested review from timothy-nunn and removed request for jonmaddock February 17, 2025 10:54
Copy link
Contributor

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where have all of the bscf and diamagnetic fraction output gone?

One of the integration test input files still has gamma in https://github.com/ukaea/PROCESS/actions/runs/13261719680/job/37019713666?pr=3520#step:7:287

Please rebase so new tests are available and Stellarator (old) is not failing

process/plasma_geometry.py Show resolved Hide resolved
process/physics.py Outdated Show resolved Hide resolved
@chris-ashe chris-ashe force-pushed the update_vscalc_function_and_add_to_docs branch 2 times, most recently from f7fd6ba to 9bc6d97 Compare February 17, 2025 11:41
@chris-ashe chris-ashe force-pushed the update_vscalc_function_and_add_to_docs branch from 9bc6d97 to f10d3d5 Compare February 17, 2025 14:15
Copy link
Contributor

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update the spherical tokamak once through with obsolete variables

https://github.com/ukaea/PROCESS/actions/runs/13372261563/job/37343429082?pr=3520#step:7:1733

Copy link
Collaborator

@j-a-foster j-a-foster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with the docs and regression tests after a discussion.

@timothy-nunn timothy-nunn merged commit d2883c4 into main Feb 17, 2025
14 of 18 checks passed
@timothy-nunn timothy-nunn deleted the update_vscalc_function_and_add_to_docs branch February 17, 2025 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Physics Relating to the physics models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants